Allow cancelling a pending video_player creation by JulienDev · Pull Request #405 · wang-bin/fvp

JulienDev · 2026-09-25T10:02:29Z

Summary

If a source stalls in MDK prepare(), createWithOptions() remains pending and VideoPlayerController.dispose() cannot release the player. I reproduced this with a localhost response delayed by 20 seconds on Windows and macOS Release.

This adds an opt-in FvpCreateCancellation scope for initialize(). Calling cancel() before dispose() lets FVP stop and delete the pending player, then return a failed ID so video_player can finish disposal. Cancellation uses FvpCreateCancelledException in both run() and the event stream. The dartdoc includes a usage example. Without the scope, stalled preparation still blocks disposal.

Player.disposeAsync() makes teardown awaitable while keeping void dispose(). MdkVideoPlayer.disposeAsync() closes the event stream and ignores late callbacks. Texture release skips the video-size wait. I kept texture allocation awaited: deleting the player while createTexture() still uses its native handle would be unsafe.

Texture creation and cleanup errors also return a failed ID, avoiding another stuck controller. Platform disposal reports release errors and retains the player for retry while its texture remains registered.

Testing

Initial reproduction project: extract it at the root of this PR checkout, then follow example/PENDING_CREATE_REPRO.md. It uses a delayed localhost response and writes MDK logs at Level.ALL. Initial Windows logs at 7b685d6: without cancellation, dispose() timed out after 8 seconds; with cancellation, it completed in 38 ms.
Flutter 3.41.9 / video_player 2.11.1: Windows Release with MDK 0.39.0 and macOS Release with MDK 0.38.0 pass cancellation, delayed texture allocation/release, invalid and disconnected sources, and stress (150 rapid switches, 20 rounds of four players, 30 rounds of four cancelled players plus one survivor). Windows stress released 380/380 players and 258/258 textures; Mac stress released 263/263 textures. The four Dart files are identical before and after the rebase onto a233309.
Both platforms: injected texture errors are reported without stuck disposal; retrying VideoPlayerPlatform.dispose(id) completes cleanup. Cancellation and normal playback are covered. Holding native texture release confirms that controller disposal waits for it.
Windows: 13 format/HTTP/HLS cases pass with default and FFmpeg decoders. DASH fails on this fixture, including on unpatched a233309 with the same MDK 0.39.0 and FFmpeg libraries.
18 Dart tests pass on each platform; analysis reports no issues. Three CMake tests pass on Windows. Mac unit tests use a callbacks.cpp loader shim; Release tests use the real plugin. Windows and macOS CI builds pass on 086395c.

The macOS retest with MDK 0.39.0 is still unconfirmed because I lost access to the test machine. I also reproduced an intermittent parallel-player teardown hang on unpatched fc97343 with the same MDK and native stack offsets. It did not occur in the completed Mac run; I haven't identified its trigger.

wang-bin · 2026-09-30T07:24:57Z

@cursor review

cursor · 2026-09-30T07:24:59Z

Bugbot couldn't run — GitHub account mismatch

The GitHub account linked to your Cursor account does not match the PR author.

Please ensure you're using the correct GitHub account, or run Bugbot from a team that covers this repository.

wang-bin

Thanks for the clear write-up and the Windows repro — the diagnosis matches the video_player create/dispose coupling (dispose waits on _creatingCompleter, which only completes after createWithOptions returns).

The overall approach is good: Zone-scoped opt-in cancellation, awaitable disposeAsync(), skipping _videoSize on texture release, and a failed player id so initialize can finish. CI looks green.

Please address the inline notes before merge. Biggest ones: align MdkVideoPlayer with disposeAsync, and clarify/simplify the Future.any + StateError vs platform error path. Also worth documenting that this remains opt-in (default initialize/dispose still hangs on a stalled prepare()).

wang-bin · 2026-09-30T07:48:41Z

+    // Resolve video_player's createWithOptions Future only after MDK has been
+    // stopped and deleted. Its controller can then safely finish dispose().
+    await player.disposeAsync();
+    player.streamCtl.close();


disposeAsync() bypasses MdkVideoPlayer.dispose(), so streamCtl has to be closed here by hand.

Prefer closing the stream controller inside an overridden disposeAsync() on MdkVideoPlayer, and keep dispose() => unawaited(disposeAsync()) so both paths tear down the same way:

@override Future<void> disposeAsync() async { if (!streamCtl.isClosed) streamCtl.close(); _initialized = false; await super.disposeAsync(); } @override void dispose() => unawaited(disposeAsync());

Then this streamCtl.close() can go away.

Done, I moved the stream cleanup into MdkVideoPlayer.disposeAsync() and kept dispose() => unawaited(disposeAsync()). I also guard late callbacks, including the textureSize continuation.

wang-bin · 2026-09-30T07:48:41Z

+              (_) => throw StateError('FVP player creation cancelled')),
+        ]),
+        zoneValues: {_createCancellationZoneKey: this},
+      );


run() completes with StateError as soon as cancel() fires, while initialize() may still be running and later surfaces a PlatformException via the cancelled event stream.

Callers awaiting run() therefore see a different error than controller.value.

Consider either:

Zone-wrap only (no Future.any throw) and let initialize fail through the normal platform error path, or

Keep the early completion but use one dedicated exception type and document that initialize's later error is expected.

Also a short usage example in fvp.dart dartdoc would help, since this is opt-in.

Thanks, I went with option 2. run() and the cancellation event now use FvpCreateCancelledException.

I kept the early completion because video_player can cancel its event subscription during dispose(), leaving the original initialize() future pending. The dartdoc explains the possible later error and that callers should await dispose() for native cleanup.

wang-bin · 2026-09-30T07:48:41Z

+    player.streamCtl.close();
+    final id = _nextFailureId--;
+    _cancelledEvents[id] = Stream<VideoEvent>.error(PlatformException(
+        code: 'creation cancelled', message: 'Playback changed'));


message: 'Playback changed' is a bit opaque (not a video_player constant I could find). Prefer something explicit like 'FVP player creation cancelled' so apps that surface errorDescription can tell cancel from a real media failure.

I changed it to FVP player creation cancelled, using the same FvpCreateCancelledException.

wang-bin · 2026-09-30T07:48:41Z

+      rethrow;
+    }
+    if (cancellation?.isCancelled ?? false) {
+      return _finishCancelledCreate(player);


Minor: prepare / textureSize are raced with _untilCancelled, but updateTexture / createTexture only check isCancelled after the await (and in catch). Cancel during texture creation is unlikely to hang like network prepare, but the handling is inconsistent — wrapping with _untilCancelled would match the earlier waits.

I kept this await because createTexture() is still using the native player handle. Racing it with cancellation would let _finishCancelledCreate() delete the player before texture creation returns.

Once allocation completes, I release the texture before deleting the player. I tested cancellation with allocation delayed by 0, 20, 200 and 1000 ms on Windows and macOS; all textures were released.

wang-bin · 2026-09-30T07:48:41Z

    if (dart.library.html) 'src/video_player_dummy.dart';

 export 'src/controller.dart';
+export 'src/create_cancellation.dart' show FvpCreateCancellation;


Please add a short usage example here (or on FvpCreateCancellation) so callers know this is opt-in and that cancel() must be called before dispose() when initialize may still be pending:

final cancel = FvpCreateCancellation(); try { await cancel.run(() => controller.initialize()); } finally { cancel.cancel(); await controller.dispose(); }

Added to the FvpCreateCancellation dartdoc, with cancel() before dispose() and a note that this is opt-in.

JulienDev · 2026-09-30T13:19:38Z

Thanks for the review! I've updated the patch in 086395c and replied inline. I also updated the test results in the PR description.

wang-bin

Re-reviewed 086395c — previous notes are addressed. LGTM.

MdkVideoPlayer.disposeAsync() now owns streamCtl cleanup; late callbacks guard streamCtl.isClosed
Shared FvpCreateCancelledException for run() and the failed-create event stream; dartdoc covers the early/initialize dual path and opt-in usage
Keeping the updateTexture await (then release in dispose) is the right call vs racing delete against an in-flight createTexture; thanks for the delayed-allocation tests on Windows/macOS
Platform dispose() awaiting disposeAsync() is a good tightening of the normal path too

CI is green on this head.

wang-bin · 2026-10-14T03:19:34Z

I add isClosed checks in master branch, can you rebase?

JulienDev · 2026-10-20T16:16:19Z

Done, rebased onto a6e59bd (v0.39.1). Your isClosed guards in 7cfe91c are the same four this PR added, so they now come from master and drop out of the diff. The rest of the patch is unchanged; analysis is clean and the 18 Dart tests pass on Windows.

wang-bin · 2026-10-21T16:27:37Z

i think you can split the patch into 3 commits like this:

Make player disposal awaitable
Return disposable IDs for create failures
Add opt-in creation cancellation

Player.dispose() was fire-and-forget, so VideoPlayerPlatform.dispose() returned before native teardown finished, and a texture release error escaped as an uncaught async error. Add Player.disposeAsync() and keep dispose() as an unawaited wrapper. MdkVideoPlayer overrides it to close its event stream first, and MdkVideoPlayerPlatform.dispose() awaits it. If the release fails, the player is kept so that dispose() can be retried. Releasing a texture no longer waits for the video size. That future never completes while a load is stalled, so disposal could hang there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

If texture creation threw, create() threw too. video_player then never got a player ID, and VideoPlayerController.dispose() kept waiting for the creation to finish. Return a failed player ID instead. Its event stream reports the error, so initialize() fails as usual, and dispose(id) releases the player kept under that ID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

If a source stalls in MDK prepare(), createWithOptions() stays pending and VideoPlayerController.dispose() waits for it forever. FvpCreateCancellation is an opt-in zone scope for initialize(). Calling cancel() before dispose() stops waiting for preparation and video size, deletes the pending player and returns a failed ID, so video_player can finish disposal. run() and the event stream both report FvpCreateCancelledException. A texture allocation already in flight is awaited, then released before the player is deleted. Without the scope, a stalled preparation still blocks disposal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

JulienDev · 2026-10-21T17:36:23Z

Done, split into three commits as you suggested:

fix: make player disposal awaitable
fix: return disposable IDs for create failures
feat: add opt-in creation cancellation

The final code is identical to 681c64e. Each commit is formatted and passes analysis; the lifecycle and disposal tests pass on each commit, and all 18 Dart tests on the last one.

wang-bin · 2026-10-22T03:49:29Z

nice

JulienDev force-pushed the fix/cancel-pending-create branch 5 times, most recently from 1c51345 to 7b685d6 Compare September 25, 2026 10:50

JulienDev marked this pull request as ready for review September 25, 2026 11:11

wang-bin requested changes Sep 30, 2026

View reviewed changes

JulienDev force-pushed the fix/cancel-pending-create branch 2 times, most recently from f462191 to 086395c Compare September 30, 2026 12:17

JulienDev requested a review from wang-bin September 30, 2026 13:36

wang-bin approved these changes Oct 14, 2026

View reviewed changes

PARD2 mentioned this pull request Oct 16, 2026

mdk.vdec0@0x16f243000 EXC_BAD_ACCESS KERN_PROTECTION_FAILURE 0x000000016f1bfff0 #410

Closed

JulienDev force-pushed the fix/cancel-pending-create branch from 086395c to 681c64e Compare October 20, 2026 16:07

JulienDev and others added 3 commits October 21, 2026 18:32

JulienDev force-pushed the fix/cancel-pending-create branch from 681c64e to c7cc94d Compare October 21, 2026 16:34

wang-bin merged commit 741d454 into wang-bin:master Oct 22, 2026
14 of 15 checks passed

Conversation

JulienDev commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

wang-bin commented Sep 30, 2026

Uh oh!

cursor Bot commented Sep 30, 2026

Bugbot couldn't run — GitHub account mismatch

Uh oh!

wang-bin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JulienDev commented Sep 30, 2026

Uh oh!

wang-bin left a comment

Choose a reason for hiding this comment

Uh oh!

wang-bin commented Oct 14, 2026

Uh oh!

JulienDev commented Oct 20, 2026

Uh oh!

wang-bin commented Oct 21, 2026

Uh oh!

JulienDev commented Oct 21, 2026

Uh oh!

Uh oh!

wang-bin commented Oct 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JulienDev commented Sep 25, 2026 •

edited

Loading